Skip to content

Conversation

@EmrysMyrddin
Copy link
Contributor

Description

Force key frame implementation for the VAAPI in both VP8 and VP9 mode.

return e
}

func (e *encoderVP8) ForceKeyFrame() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand, this function method already exists and returns the encoder itself.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also don't remember what I wanted to point and old CI log is gone.
The interface seems be correct.

@codecov
Copy link

codecov bot commented Feb 1, 2023

Codecov Report

Patch has no changes to coverable lines.

📢 Thoughts on this report? Let us know!.

}

func (e *encoderVP8) ForceKeyFrame() {
e.forceKeyFrame = true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may cause data race if ForceKeyFrame() is called from other goroutine during Read()?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've changed the type to use an atomic.Bool instead. This way, there is no data race possible.

e.frParam.data.framerate = C.uint(e.rate.Calc())

if kf {
if kf || e.forceKeyFrame {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

forceKeyFrame isn't reset, so every frames after ForceKeyFrame() will be keyframe.

@edaniels
Copy link
Member

edaniels commented Mar 3, 2023

Hi @EmrysMyrddin. It looks like this one is close. Do you still want to get this in?

@EmrysMyrddin EmrysMyrddin force-pushed the feature/force_key_frame_vaapi branch from 8eb5e86 to 410e856 Compare September 20, 2023 19:33
@EmrysMyrddin
Copy link
Contributor Author

@edaniels I've merged master and applied suggestions if you want :-)

Copy link
Member

@edaniels edaniels left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

@edaniels edaniels merged commit 2882fd4 into master Sep 20, 2023
@edaniels edaniels deleted the feature/force_key_frame_vaapi branch September 20, 2023 19:46

rate *framerateDetector

forceKeyFrame atomic.Bool
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to bump minimal supported go version to 1.19

go 1.13

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants